Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dnsproxy: Add DNS proxying functionality. #40

Closed
wants to merge 1 commit into from
Closed

Conversation

hedss
Copy link
Contributor

@hedss hedss commented Aug 27, 2019

There are services that don't use the libc resolver
in their service containers (for example some Go-based
services). This addition installs dnsmasq into the
MDNS services which can be enabled by an envvar.

Change-type: minor
Signed-off-by: Heds Simons [email protected]

@hedss hedss requested review from Page- and wrboyce as code owners August 27, 2019 15:15
@hedss hedss force-pushed the dns-proxying branch 2 times, most recently from f96358e to 88db097 Compare August 27, 2019 15:18
@hedss hedss requested a review from xginn8 August 27, 2019 15:19
@hedss hedss changed the title dnsproxy: Add DNS proxying functionality. WIP: dnsproxy: Add DNS proxying functionality. Aug 27, 2019
@hedss
Copy link
Contributor Author

hedss commented Aug 27, 2019

Don't review yet, there's an issue which needs sorting.

@nazrhom
Copy link

nazrhom commented Aug 27, 2019

@balena-ci retest

@hedss hedss changed the title WIP: dnsproxy: Add DNS proxying functionality. dnsproxy: Add DNS proxying functionality. Aug 29, 2019
@hedss
Copy link
Contributor Author

hedss commented Aug 29, 2019

Unforuntately, balenaCI is currently producing broken builds of this PR. We can't merge it till this is rectified.

@hedss
Copy link
Contributor Author

hedss commented Aug 29, 2019

@balena-ci retest

There are services that don't use the libc resolver
in their service containers (for example some Go-based
services).

This feature allows the MDNS publisher to act as a
DNS proxy on bridged networks for these services.

Set the 'PROXY_DNS' envvar to 'true' to proxy DNS
instead of publishing MDNS records, and ensure the
networking mode is the bridge network and not 'host'.

Change-type: minor
Signed-off-by: Heds Simons <[email protected]>
@nazrhom
Copy link

nazrhom commented Sep 2, 2019

@balena-ci retest

1 similar comment
@nazrhom
Copy link

nazrhom commented Sep 2, 2019

@balena-ci retest

xginn8
xginn8 previously approved these changes Sep 10, 2019
@@ -15,6 +16,7 @@ RUN JOBS=MAX npm ci --unsafe-perm --production && npm cache clean --force && rm

# Copy and enable the service
COPY config/services /etc/systemd/system
RUN systemctl disable dnsmasq.service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we never want dnsmasq running here, it might be slightly safer to mask the unit (preventing anything else from pulling in dnsmasq as a dep)

return;
}

console.log(`Removing ${hostname} at address from local MDNS pool`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing the ${address} here

export async function startMdnsPublisher(): Promise<void> {
const tld = process.env.MDNS_TLD;
if (!tld) {
throw new Error('MDNS_TLD must be set!');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new Error('MDNS_TLD must be set!');
throw new Error('MDNS_TLD must be set in the environment!');

const hosts = getFullHostnames();

try {
const ipAddr = await getHostAddress(process.env.INTERFACE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This address could potentially change over the lifecycle of BoB, should this value be refreshed on a timer of some sort (see #35)?

Also, this may be completely out-of-scope for this change, but the way this diff is rendered makes it exceedingly difficult to tell new code from old (in which case, ignore!)

@kaisoz kaisoz added the versionbot/pr-draft Draft PR - Don't merge this PR automatically label Jun 10, 2020
@ghost ghost dismissed xginn8’s stale review June 10, 2020 14:21

Approval reviews not allowed in Draft PRs

@ab77 ab77 closed this Jun 13, 2022
@jellyfish-bot jellyfish-bot reopened this Jun 13, 2022
@ab77 ab77 closed this Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
versionbot/pr-draft Draft PR - Don't merge this PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants